Skip to content

Add test for FetchContent of UMF on Linux and Windows - #1384

Closed
ldorau wants to merge 1 commit into
oneapi-src:mainfrom
ldorau:Test_FetchContent_of_UMF_on_Linux_and_Windows
Closed

Add test for FetchContent of UMF on Linux and Windows#1384
ldorau wants to merge 1 commit into
oneapi-src:mainfrom
ldorau:Test_FetchContent_of_UMF_on_Linux_and_Windows

Conversation

@ldorau

@ldorau ldorau commented Jun 23, 2025

Copy link
Copy Markdown
Contributor

Description

Test FetchContent of UMF on Linux and Windows -
on Windows with the 'Ninja' and 'NMake Makefiles' generators.

Add the fetch_content example based on the basic example.

Nightly CI build: https://github.com/ldorau/unified-memory-framework/actions/runs/16030235338

Checklist

  • Code compiles without errors locally
  • All tests pass locally
  • CI workflows execute properly

@ldorau
ldorau requested a review from a team as a code owner June 23, 2025 11:23
@ldorau
ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch 4 times, most recently from afd45b5 to cd0640d Compare June 23, 2025 12:10
@ldorau

ldorau commented Jun 23, 2025

Copy link
Copy Markdown
Contributor Author

@ldorau

ldorau commented Jun 24, 2025

Copy link
Copy Markdown
Contributor Author

@ldorau
ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from cd0640d to 09fb801 Compare June 24, 2025 11:08
@ldorau

ldorau commented Jun 24, 2025

Copy link
Copy Markdown
Contributor Author

@ldorau
ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from 09fb801 to 8bb3ebc Compare June 27, 2025 09:09
@ldorau

ldorau commented Jun 27, 2025

Copy link
Copy Markdown
Contributor Author

Rebased

@ldorau
ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from 8bb3ebc to 9906125 Compare June 30, 2025 07:09
@ldorau

ldorau commented Jun 30, 2025

Copy link
Copy Markdown
Contributor Author

Rebased

@ldorau ldorau changed the title Test FetchContent of UMF on Linux and Windows Add tests for FetchContent of UMF on Linux and Windows Jun 30, 2025
@ldorau
ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from 9906125 to 4e4194a Compare June 30, 2025 07:51
Comment thread .github/workflows/reusable_fetch_content.yml Outdated
Comment thread .github/workflows/reusable_fetch_content.yml Outdated
Comment thread .github/workflows/reusable_fetch_content.yml Outdated
Comment thread .github/workflows/reusable_fetch_content.yml Outdated
Comment thread .github/workflows/reusable_fetch_content.yml Outdated
Comment thread examples/basic/find_umf.cmake Outdated
Comment thread examples/basic/include.cmake Outdated
Comment thread examples/basic/CMakeLists.txt Outdated
Comment thread examples/basic/include.cmake Outdated
Comment thread examples/fetch_content/find_umf.cmake
@ldorau
ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from 4e4194a to 64a6257 Compare June 30, 2025 10:16
@ldorau
ldorau requested a review from bratpiorka June 30, 2025 10:18
@ldorau ldorau changed the title Add tests for FetchContent of UMF on Linux and Windows Add test for FetchContent of UMF on Linux and Windows Jun 30, 2025
Comment thread .github/workflows/pr_push.yml Outdated
Comment thread .github/workflows/reusable_fetch_content.yml Outdated
Comment thread examples/fetch_content/fetch_umf.cmake Outdated
Comment thread examples/fetch_content/fetch_umf.cmake
@ldorau
ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from 64a6257 to b28de77 Compare July 2, 2025 13:42
@ldorau

ldorau commented Jul 2, 2025

Copy link
Copy Markdown
Contributor Author

@ldorau
ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from b28de77 to e2286df Compare July 2, 2025 16:07
@ldorau
ldorau requested a review from lukaszstolarczuk July 2, 2025 16:14

@lukaszstolarczuk lukaszstolarczuk left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in general LGTM

Comment thread .github/workflows/nightly.yml Outdated
Comment thread .github/workflows/nightly.yml Outdated
Comment thread .github/workflows/nightly.yml Outdated
${{ matrix.umfd_lib == 'ON' && '--umfd-lib' || ''}}
${{ matrix.static_hwloc == 'ON' && '--hwloc' || '' }}

- name: Set VCPKG_PATH with hwloc for the fetch_content example

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we already have a step like this, I guess we could just do the same as in that step: add if: matrix.static_hwloc == 'OFF' to the fetch_content example's configure/build/execution steps

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fetch_content example always requires hwloc

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I'm just saying, we could test that example only in cases where the job already have VCPKG_PATH set

# Copyright (C) 2025 Intel Corporation
# Under the Apache License v2.0 with LLVM Exceptions. See LICENSE.TXT.
# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could add a short description why do we have this example

Comment thread examples/fetch_content/fetch_umf.cmake Outdated
Comment thread examples/fetch_content/fetch_umf.cmake
Comment thread .github/workflows/nightly.yml Outdated
target_include_directories(${EXAMPLE_NAME} PRIVATE ${LIBUMF_INCLUDE_DIRS})
target_link_directories(${EXAMPLE_NAME} PRIVATE ${LIBHWLOC_LIBRARY_DIRS})
target_link_libraries(${EXAMPLE_NAME} PRIVATE ${LIBUMF_LIBRARIES} hwloc)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please add printing of PATHs found in this example and used above..?

// e.g. like here: https://github.com/lukaszstolarczuk/unified-memory-framework/actions/runs/16051712582/job/45295762065#step:11:4353

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which paths exactly do you mean?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mostly which umf is used (so LIBUMF_INCLUDE_DIRS, but also btw we could print LIBHWLOC_LIBRARY_DIRS and LIBUMF_LIBRARIES) - see my example CI run (link above)

@ldorau
ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from e2286df to c666f12 Compare July 4, 2025 08:04
@ldorau
ldorau requested a review from lukaszstolarczuk July 4, 2025 08:09
Test FetchContent of UMF on Linux and Windows in Nightly CI job
- on Windows with the 'Ninja' and 'NMake Makefiles' generators.

Add the fetch_content example based on the basic example.

Signed-off-by: Lukasz Dorau <lukasz.dorau@intel.com>
@ldorau
ldorau force-pushed the Test_FetchContent_of_UMF_on_Linux_and_Windows branch from c666f12 to 1a033d4 Compare July 4, 2025 13:00
@ldorau

ldorau commented Jul 4, 2025

Copy link
Copy Markdown
Contributor Author

CACHE INTERNAL "UMF_LINK_HWLOC_STATICALLY=OFF")

FetchContent_MakeAvailable(unified-memory-framework)
FetchContent_GetProperties(unified-memory-framework)

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just verified this FetchContent_GetProperties call is redundant - MakeAvailable should be enough.

@lukaszstolarczuk

Copy link
Copy Markdown
Collaborator

as @ldorau is off for a moment, contd. in #1432

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants